fix: restore table height virtualization classname (BED-9057) - #3054
fix: restore table height virtualization classname (BED-9057)#3054dcairnsspecterops wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesExploreTable styling
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx`:
- Line 226: Update the ExploreTable wrapper around MemoDataTable so its height
reflects isExpanded: use the expanded available-height calculation when expanded
and retain the collapsed viewport height otherwise, or allow the parent
grid/ScrollArea to determine it. Ensure virtualization receives the correct
height in both states and add coverage for collapsed and expanded rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: dc190072-a5db-4d28-b65c-c779f9394105
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx
| onColumnOrderChange={(newOrder) => { | ||
| setColumnOrder(newOrder); | ||
| }} | ||
| className='overflow-auto h-[calc(50vh-72px)]' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the table height responsive to expanded mode.
The wrapper expands from h-1/2 to h-[calc(100%-72px)], but MemoDataTable always receives h-[calc(50vh-72px)]. In expanded mode, virtualization can therefore measure only the collapsed-height viewport, leaving unused space and incorrect scrolling behavior. Make the height conditional on isExpanded or let the surrounding grid/ScrollArea provide the available height, and add coverage for both states.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx`
at line 226, Update the ExploreTable wrapper around MemoDataTable so its height
reflects isExpanded: use the expanded available-height calculation when expanded
and retain the collapsed viewport height otherwise, or allow the parent
grid/ScrollArea to determine it. Ensure virtualization receives the correct
height in both states and add coverage for collapsed and expanded rendering.
There was a problem hiding this comment.
Wow, smart bunny
There was a problem hiding this comment.
@dcairnsspecterops, happy to help! 🐇✨
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx`:
- Around line 225-229: Update the height-class conditions in the ExploreTable
container’s cn call so the 100vh height uses isExpanded instead of repeating
!isExpanded. Preserve the existing collapsed height condition and ensure exactly
one viewport height class applies in each state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d8072125-f782-4b5d-adcd-9b0adf6285d0
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx
| className={cn('overflow-auto', { | ||
| // TODO: why is header disappearing on bottom scroll? One extra overflow element somewhere | ||
| 'h-[calc(50vh-72px)]': !isExpanded, | ||
| 'h-[calc(100vh-72px)]': !isExpanded, | ||
| })} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the expanded-height condition.
Line 228 repeats !isExpanded; twMerge leaves collapsed mode at h-[calc(100vh-72px)], while expanded mode gets no height class. This breaks the virtualization viewport in both states. Use isExpanded on line 228.
Proposed fix
className={cn('overflow-auto', {
// TODO: why is header disappearing on bottom scroll? One extra overflow element somewhere
'h-[calc(50vh-72px)]': !isExpanded,
- 'h-[calc(100vh-72px)]': !isExpanded,
+ 'h-[calc(100vh-72px)]': isExpanded,
})}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx`
around lines 225 - 229, Update the height-class conditions in the ExploreTable
container’s cn call so the 100vh height uses isExpanded instead of repeating
!isExpanded. Preserve the existing collapsed height condition and ensure exactly
one viewport height class applies in each state.
jvacca-specterops
left a comment
There was a problem hiding this comment.
Thanks for catching this!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx`:
- Line 225: Move the `overflow-auto h-full` classes from the `className` prop on
the table to `TableProps.tableContainerClassName`. Keep the table’s own styling
separate, and ensure `disableDefaultOverflowAuto` remains configured so the
virtualization viewport stays within the outer `overflow-hidden` container.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 3e5043e2-6f7a-4194-a4e1-e1180e16b944
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx
| onColumnOrderChange={(newOrder) => { | ||
| setColumnOrder(newOrder); | ||
| }} | ||
| className='overflow-auto h-full' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ExploreTable references ==\n'
rg -n "MemoDataTable|tableContainerClassName|className='overflow-auto h-full'|overflow-hidden" packages/javascript/bh-shared-ui/src/components/ExploreTable packages/javascript/bh-shared-ui/src/components -g '*.tsx' -g '*.ts'
printf '\n== ExploreTable file excerpt ==\n'
sed -n '180,260p' packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx
printf '\n== Table component definitions ==\n'
fd -a "DataTable" packages/javascript/bh-shared-ui/src/components
fd -a "TableProps" packages/javascript/bh-shared-ui/src/components
printf '\n== likely table implementation excerpts ==\n'
for f in $(rg -l "tableContainerClassName|MemoDataTable|className" packages/javascript/bh-shared-ui/src/components -g '*.tsx' -g '*.ts'); do
echo "--- $f"
sed -n '1,260p' "$f" | rg -n "tableContainerClassName|MemoDataTable|className|overflow-auto|overflow-hidden|table" -n -C 3
doneRepository: SpecterOps/BloodHound
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DataTable outline =="
ast-grep outline packages/javascript/bh-shared-ui/src/components/DataTable.tsx --view expanded
echo
echo "== DataTable relevant excerpt =="
sed -n '1,260p' packages/javascript/bh-shared-ui/src/components/DataTable.tsx | rg -n "tableContainerClassName|className|TableContainer|TableProps|virtualizationOptions|Memo|container" -C 4
echo
echo "== TableProps / prop forwarding in ExploreTable =="
sed -n '1,260p' packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx | rg -n "tableProps|TableProps|tableContainerClassName|className='overflow-auto h-full'|MemoDataTable" -C 4Repository: SpecterOps/BloodHound
Length of output: 6501
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DataTable outline =="
ast-grep outline packages/javascript/bh-shared-ui/src/components/DataTable.tsx --view expanded
echo
echo "== DataTable prop declarations and JSX =="
sed -n '1,260p' packages/javascript/bh-shared-ui/src/components/DataTable.tsx | nl -ba | sed -n '1,260p' | rg -n "tableContainerClassName|className|TableContainer|TableProps|virtualizationOptions|TableHead|TableBody|Table" -C 5
echo
echo "== ExploreTable props setup =="
sed -n '180,250p' packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx | nl -ba | sed -n '1,120p'Repository: SpecterOps/BloodHound
Length of output: 712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DataTable class forwarding =="
rg -n "tableContainerClassName|className|TableContainer|TableHead|TableBody|TableProps" packages/javascript/bh-shared-ui/src/components/DataTable.tsx -C 3
echo
echo "== DataTable excerpt around return JSX =="
nl -ba packages/javascript/bh-shared-ui/src/components/DataTable.tsx | sed -n '1,260p' | sed -n '1,260p' | awk 'NR>=1 && NR<=260 {print}' | rg -n "^[0-9]+[[:space:]]+return|TableContainer|className=|tableContainerClassName|Table" -C 6
echo
echo "== ExploreTable tableProps setup =="
rg -n "const tableProps|tableProps =|TableProps|tableContainerClassName" packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx -C 5Repository: SpecterOps/BloodHound
Length of output: 2548
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' packages/javascript/bh-shared-ui/src/components/DataTable.tsx | cat -n | sed -n '1,260p'Repository: SpecterOps/BloodHound
Length of output: 4707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DataTable.tsx core JSX =="
nl -ba packages/javascript/bh-shared-ui/src/components/DataTable.tsx | sed -n '1,240p'
echo
echo "== ExploreTable.tsx table props block =="
nl -ba packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx | sed -n '150,240p'Repository: SpecterOps/BloodHound
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "tableContainerClassName|TableProps|className" packages/javascript/bh-shared-ui/src/components/DataTable.tsx packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx -C 2Repository: SpecterOps/BloodHound
Length of output: 10839
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ExploreTable import block =="
sed -n '1,80p' packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx | cat -n
echo
echo "== DataTable.tsx full prop surface =="
sed -n '1,220p' packages/javascript/bh-shared-ui/src/components/DataTable.tsx | cat -nRepository: SpecterOps/BloodHound
Length of output: 8067
🌐 Web query:
doodle-ui DataTable TableProps className tableContainerClassName disableDefaultOverflowAuto
💡 Result:
There is no evidence of a technical library or component called doodle-ui that contains a DataTable component with the specific props className, tableContainerClassName, or disableDefaultOverflowAuto. The name doodle-ui is associated with multiple distinct, unrelated projects, none of which appear to be a mainstream React component library featuring a DataTable with these specific API signatures: 1. A Python-based GPU-accelerated UI and game engine [1]. 2. A minimalistic UI component library [2]. 3. A frontend library focusing on UI functionality, sometimes referred to as DodleUI [3]. 4. A collection of packages on JSR (@doodleui), such as panda-preset and styled-system, which do not document a DataTable component [4][5]. The props you mentioned (className, tableContainerClassName, disableDefaultOverflowAuto) are common patterns in various React UI component libraries (such as those based on Tailwind CSS or TanStack Table wrappers), but they are not part of a standardized API for a known library named doodle-ui. It is likely these props belong to a private, internal project, a less-documented library, or a misunderstanding of a different component library's API.
Citations:
- 1: https://pypi.org/project/doodle-ui/1.2.5/
- 2: https://github.com/siyabendoezdemir/DoodleUI
- 3: https://github.com/mosefatullah/doodleui
- 4: https://jsr.io/@doodleui
- 5: https://jsr.io/@doodleui/styled-system/doc/all_symbols
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "tableContainerClassName|disableDefaultOverflowAuto|interface DataTableProps|className.*TableProps|doodle-ui" . -g '*.ts' -g '*.tsx' -g '*.d.ts' -g '*.md' -g 'package.json'Repository: SpecterOps/BloodHound
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== doodle-ui DataTable outline =="
ast-grep outline packages/javascript/doodle-ui/src/components/DataTable/DataTable.tsx --view expanded
echo
echo "== doodle-ui DataTable relevant excerpt =="
sed -n '1,380p' packages/javascript/doodle-ui/src/components/DataTable/DataTable.tsx | cat -n | sed -n '1,380p' | rg -n "TableProps|tableContainerClassName|disableDefaultOverflowAuto|className|TableContainer|Table" -C 4
echo
echo "== doodle-ui Table component =="
sed -n '1,180p' packages/javascript/doodle-ui/src/components/Table/Table.tsx | cat -nRepository: SpecterOps/BloodHound
Length of output: 17344
Move overflow-auto h-full onto TableProps.tableContainerClassName. className styles the <table> itself; the scrollable wrapper is controlled by tableContainerClassName/disableDefaultOverflowAuto, so the virtualization viewport stays inside the outer overflow-hidden container.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/javascript/bh-shared-ui/src/components/ExploreTable/ExploreTable.tsx`
at line 225, Move the `overflow-auto h-full` classes from the `className` prop
on the table to `TableProps.tableContainerClassName`. Keep the table’s own
styling separate, and ensure `disableDefaultOverflowAuto` remains configured so
the virtualization viewport stays within the outer `overflow-hidden` container.
Description
It looks like we lost a fixed height and overflow-scroll somewhere along the way, and DataTable virtualization depends on these properties. This PR restores those CSS properties and removes the custom scrollbars, to be restored in a later ticket (https://specterops.atlassian.net/browse/BED-9074)
Screen.Recording.2026-07-24.at.5.34.53.PM.mov
Describe your changes in detail
Motivation and Context
Resolves BED-9057
Why is this change required? What problem does it solve?
How Has This Been Tested?
Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
Summary of changes